Fix Cosmos enable_cache setting #1025

tatiana · 2024-06-05T10:27:39Z

As of Cosmos 1.4.1, users are not able to disable the cache in Cosmos using AIRFLOW__COSMOS__ENABLE_CACHE=0.

During a recent refactoring #975, the enable_cache was changed to a non-boolean config. This PR fixes this issue.

During refactoring, it was changed to a non-boolean, not allowing users to disable the cache in Cosmos using AIRFLOW__COSMOS__ENABLE_CACHE=0

netlify · 2024-06-05T10:28:06Z

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name	Link
🔨 Latest commit	`cf36cc7`
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/66603d9dadfc8d00083c93b6

pankajastro

Thanks for fixing it!

As of Cosmos 1.4.1, users are not able to disable the cache in Cosmos using `AIRFLOW__COSMOS__ENABLE_CACHE=0`. During a recent refactoring #975, the `enable_cache` was changed to a non-boolean config. This PR fixes this issue.

@marco9663

Bug fixes * Fix the invocation mode for ``ExecutionMode.VIRTUALENV`` by @marco9663 in #1023 * Fix Cosmos ``enable_cache`` setting by @tatiana in #1025 * Make ``GoogleCloudServiceAccountDictProfileMapping`` dataset profile arg optional by @oliverrmaa and @pankajastro in #839 and #1017 * Athena profile mapping set ``aws_session_token`` in profile only if it exists by @pankajastro in #1022 Others * Update dbt and Airflow conflicts matrix by @tatiana in #1026 * Enable Python 3.12 unittest by @pankajastro in #1018 * Improve error logging in ``DbtLocalBaseOperator`` by @davidsteinar in #1004 * Add GitHub issue templates for bug reports and feature request by @pankajkoti in #1009 * Add more fields in bug template to reduce turnaround in issue triaging by @pankajkoti in #1027 * Fix ``dev/Dockerfile`` + Add ``uv pip install`` for faster build time by @dwreeves in #997 * Drop support for Airflow 2.3 by @pankajkoti in #994 * Update Astro Runtime image by @RNHTTR in #988 and #989 * Enable ruff F linting by @pankajastro in #985 * Move Cosmos Airflow configuration to settings.py by @pankajastro in #975 * Fix CI Issues by @tatiana in #1005 * Pre-commit hook updates in #1000, #1019 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Reeves <31971762+dwreeves@users.noreply.github.com> Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com> Co-authored-by: davidsteinar <davidsteinar@gmail.com> Co-authored-by: David Steinar Asgrimsson <david-steinar.asgrimsson@kommuninvest.se> Co-authored-by: Marco Yuen <33394715+marco9663@users.noreply.github.com> Co-authored-by: Pankaj Singh <98807258+pankajastro@users.noreply.github.com> Co-authored-by: Ollie Ma <oliver.zheyi.ma@gmail.com>

maslick · 2024-06-11T12:42:47Z

@tatiana Does AIRFLOW__COSMOS__ENABLE_CACHE=False work in 1.4.1?

tatiana · 2024-06-11T13:26:43Z

@maslick There was an issue in the 1.4.1 release with this configuration, this was solved and is available on 1.4.2 and 1.4.3

maslick · 2024-06-11T13:29:47Z

@tatiana I understand. The reason I am asking is that we're using AIRFLOW__COSMOS__ENABLE_CACHE=False (and not AIRFLOW__COSMOS__ENABLE_CACHE=0 as in the description of this PR) with 1.4.1, but we can't easily migrate to 1.4.2/1.4.3 at the moment. So my question is, would no caching even work with 1.4.1 (AIRFLOW__COSMOS__ENABLE_CACHE=False)?

tatiana · 2024-06-11T13:37:30Z

@maslick In Cosmos 1.4.1, if you want to disable the cache, you can do this via

export AIRFLOW__COSMOS__ENABLE_CACHE=''

If you check this PR, we fixed how we retrieve the envvar to use Airflow conf.getboolean:

enable_cache = conf.getboolean("cosmos", "enable_cache", fallback=True)

In Cosmos 1.4.1, this was implemented conf.get by mistake.

From Cosmos 1.4.2, both AIRFLOW__COSMOS__ENABLE_CACHE=False as well as AIRFLOW__COSMOS__ENABLE_CACHE=0 will work.

Did you have any issues with cache that made you want to switch off this feature?

maslick · 2024-06-11T13:53:33Z

@tatiana our issue with cache enabled was that it quickly fills up the 30 GB storage volume on MWAA workers. We are running a large number of DAGs (>500) with lots of dbt models/transformations... We can't increase the storage volume on MWAA either. That's why we decided to disable cache entirely. Of course, this would increase the parsing times...

josefbits · 2024-06-11T14:49:52Z

@tatiana With cache disabled it's no longer possible to do partial parsing from a supplied manifest. Is this intended behavior and or is there some config that would need to change to get it working?

tatiana · 2024-06-11T21:29:47Z

@maslick this issue will hopefully be resolved once #927 is implemented. I'm sorry that the cache had such a massive impact on your deployment - we weren't expecting for that to take 30 GB volume storage. It's a shame MWAA doesn't handle better temp files.

@josefbits I believe this is unintended behaviour. Please open an issue using https://github.com/astronomer/astronomer-cosmos/issues, and we'll follow up on this ASAP. Contributions are also welcome; it should be a small fix.

josefbits · 2024-06-11T22:21:05Z

@tatiana Thanks, I've opened #1041 for this. I'll take a look at it more tomorrow, but maybe someone with more knowledge of the codebase can figure it out quicker.

josefbits · 2024-06-11T22:55:37Z

@tatiana Not sure if this is the right place to continue this discussion, but with regards to

@maslick this issue will hopefully be resolved once #927 is implemented. I'm sorry that the cache had such a massive impact on your deployment - we weren't expecting for that to take 30 GB volume storage. It's a shame MWAA doesn't handle better temp files.

Is there some specific reason that you need to cache multiple copies of the manifests? My best guess was in case of write locks by multiple Airflow tasks. Right now Cosmos is caching once per DbtTaskGroup and for projects with a large number of models, dags, task groups etc it can become problematic on managed Airflow solutions.

Speaking of dbt parsing specifically, I've done a bunch of comparisons on the manifest.json before and after run, with partial parse enabled/disabled, and different methods to change the dbt state. From what I've seen the only thing that really changes is metadata like created_at, raw_code, and compile/build paths. This is, of course, assuming parity between CI build environment and run time, and that one is not doing something strange that might change the model code or dependencies.

Just looking for a bit more insight :)

tatiana · 2024-06-12T07:37:11Z

@josefbits, thanks for your insights!
This is a better place to discuss this: #1042

The issue we're discussing was not introduced by this PR, which was already merged last week - so new discussions may get lost.

As of Cosmos 1.4.1, users are not able to disable the cache in Cosmos using `AIRFLOW__COSMOS__ENABLE_CACHE=0`. During a recent refactoring astronomer#975, the `enable_cache` was changed to a non-boolean config. This PR fixes this issue.

@marco9663

Bug fixes * Fix the invocation mode for ``ExecutionMode.VIRTUALENV`` by @marco9663 in astronomer#1023 * Fix Cosmos ``enable_cache`` setting by @tatiana in astronomer#1025 * Make ``GoogleCloudServiceAccountDictProfileMapping`` dataset profile arg optional by @oliverrmaa and @pankajastro in astronomer#839 and astronomer#1017 * Athena profile mapping set ``aws_session_token`` in profile only if it exists by @pankajastro in astronomer#1022 Others * Update dbt and Airflow conflicts matrix by @tatiana in astronomer#1026 * Enable Python 3.12 unittest by @pankajastro in astronomer#1018 * Improve error logging in ``DbtLocalBaseOperator`` by @davidsteinar in astronomer#1004 * Add GitHub issue templates for bug reports and feature request by @pankajkoti in astronomer#1009 * Add more fields in bug template to reduce turnaround in issue triaging by @pankajkoti in astronomer#1027 * Fix ``dev/Dockerfile`` + Add ``uv pip install`` for faster build time by @dwreeves in astronomer#997 * Drop support for Airflow 2.3 by @pankajkoti in astronomer#994 * Update Astro Runtime image by @RNHTTR in astronomer#988 and astronomer#989 * Enable ruff F linting by @pankajastro in astronomer#985 * Move Cosmos Airflow configuration to settings.py by @pankajastro in astronomer#975 * Fix CI Issues by @tatiana in astronomer#1005 * Pre-commit hook updates in astronomer#1000, astronomer#1019 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Reeves <31971762+dwreeves@users.noreply.github.com> Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com> Co-authored-by: davidsteinar <davidsteinar@gmail.com> Co-authored-by: David Steinar Asgrimsson <david-steinar.asgrimsson@kommuninvest.se> Co-authored-by: Marco Yuen <33394715+marco9663@users.noreply.github.com> Co-authored-by: Pankaj Singh <98807258+pankajastro@users.noreply.github.com> Co-authored-by: Ollie Ma <oliver.zheyi.ma@gmail.com>

Fix Cosmos enable_cache setting

cf36cc7

During refactoring, it was changed to a non-boolean, not allowing users to disable the cache in Cosmos using AIRFLOW__COSMOS__ENABLE_CACHE=0

dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 5, 2024

tatiana temporarily deployed to internal June 5, 2024 10:27 — with GitHub Actions Inactive

dosubot bot added area:config Related to configuration, like YAML files, environment variables, or executer configuration area:testing Related to testing, like unit tests, integration tests, etc labels Jun 5, 2024

pankajastro approved these changes Jun 5, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 5, 2024

pankajkoti approved these changes Jun 5, 2024

View reviewed changes

tatiana merged commit 0e4ca97 into main Jun 5, 2024
61 checks passed

tatiana deleted the fix-bool-setting branch June 5, 2024 10:40

tatiana mentioned this pull request Jun 6, 2024

Release 1.4.2 #1028

Merged

tatiana mentioned this pull request Jun 11, 2024

[Bug] Cosmos stale temporary directories #958

Open

4 tasks

tatiana mentioned this pull request Jun 12, 2024

[Bug] Caching filling up Airflow nodes disk #1042

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Cosmos enable_cache setting #1025

Fix Cosmos enable_cache setting #1025

tatiana commented Jun 5, 2024

netlify bot commented Jun 5, 2024 •

edited

Loading

pankajastro left a comment

maslick commented Jun 11, 2024 •

edited

Loading

tatiana commented Jun 11, 2024 •

edited

Loading

maslick commented Jun 11, 2024 •

edited

Loading

tatiana commented Jun 11, 2024

maslick commented Jun 11, 2024 •

edited

Loading

josefbits commented Jun 11, 2024 •

edited

Loading

tatiana commented Jun 11, 2024 •

edited

Loading

josefbits commented Jun 11, 2024 •

edited

Loading

josefbits commented Jun 11, 2024 •

edited

Loading

tatiana commented Jun 12, 2024

Fix Cosmos enable_cache setting #1025

Fix Cosmos enable_cache setting #1025

Conversation

tatiana commented Jun 5, 2024

netlify bot commented Jun 5, 2024 • edited Loading

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

pankajastro left a comment

Choose a reason for hiding this comment

maslick commented Jun 11, 2024 • edited Loading

tatiana commented Jun 11, 2024 • edited Loading

maslick commented Jun 11, 2024 • edited Loading

tatiana commented Jun 11, 2024

maslick commented Jun 11, 2024 • edited Loading

josefbits commented Jun 11, 2024 • edited Loading

tatiana commented Jun 11, 2024 • edited Loading

josefbits commented Jun 11, 2024 • edited Loading

josefbits commented Jun 11, 2024 • edited Loading

tatiana commented Jun 12, 2024

netlify bot commented Jun 5, 2024 •

edited

Loading

maslick commented Jun 11, 2024 •

edited

Loading

tatiana commented Jun 11, 2024 •

edited

Loading

maslick commented Jun 11, 2024 •

edited

Loading

maslick commented Jun 11, 2024 •

edited

Loading

josefbits commented Jun 11, 2024 •

edited

Loading

tatiana commented Jun 11, 2024 •

edited

Loading

josefbits commented Jun 11, 2024 •

edited

Loading

josefbits commented Jun 11, 2024 •

edited

Loading